-
Notifications
You must be signed in to change notification settings - Fork 145
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
cassandane: support multiple output formats per run #4970
Conversation
This disables all output formats except 'tap' for now. They'll be added back as the old RunnerFoos are converted to FormatFoos
use vars qw($VERSION); | ||
# XXX should this inherit from our own Cassandane::Unit::Runner? | ||
use base qw(Test::Unit::Runner); | ||
use lib '.'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this being added? Feels like a code smell. The contents of @INC
should probably be correct by now, unless I don't know how this works — which is possible!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cassandane expects to run out of its own source tree, which means none of its modules are installed to anywhere @INC
looks, so all of Cassandane needs to use lib '.';
before the use lines for its own stuff. See d03d49b
RunnerXML didn't already have this line because it was an oddball that didn't use any Cassandane modules, but now it does so it does.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This (use lib '.'
) is the kind of thing I'd expect in the program that uses the modules, not the modules themselves. For example, testrunner.pl
. Then you could delete it from many other places.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Like many heroic acts, I'm glad you did this and sorry it was required. 🤪
This looks like a nice clean refactoring and addition. I still really dislike the underlying code, but we gotta make do with what we have sometimes! This cleaned it up a little, which is nice.
I have not yet attempted to run this, only read it. I will give it a run tomorrowish.
This adds support to Cassandane for producing multiple formats of output from a single run, as long as they won't be fighting over stdout. In practice, this means "tap", "pretty", and "prettier" still cannot be used together, but "xml" can be used alongside any of them. This will become more useful if we add more non-stdout formats in the future (perhaps "json"), but this PR doesn't do that.
There's a bunch of internal architectural changes here, but the main outwardly-visible one should be that you can now specify the
-f <format>
argument to testrunner.pl multiple times. The other is that the--rerun
behaviour after test failures now works independently of output format(s).Reviewers: Please pull this branch and run Cassandane in any of the ways you usually would. If you can test this in an FM inabox or build system setup, even better. I'm confident that I haven't broken any of my own use cases, but less confident about use cases I don't use myself. The diff probably makes the most sense when read commit-by-commit.